test(state): add unit tests for state accessors#3752
Conversation
There was a problem hiding this comment.
Pull request overview
Adds dedicated unit tests for core/state/accessors.go to address missing direct coverage (Issue #3688), validating basic CRUD behavior for contracts/classes and correctness of history accessors when read through StateReader.
Changes:
- Adds unit tests for contract and class accessors (
Has/Get/Write/Delete*). - Adds tests for nonce/class-hash/storage history accessors using
StateReaderhistorical reads. - Adds tests for
GetStateObjectnot-found propagation and success wrapping.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c13deef to
24e1f7f
Compare
|
My mistake, were address but not answered |
|
@rodrodros all comments are fixed |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3752 +/- ##
==========================================
+ Coverage 75.03% 76.36% +1.33%
==========================================
Files 433 433
Lines 38594 38631 +37
==========================================
+ Hits 28960 29502 +542
+ Misses 7632 7062 -570
- Partials 2002 2067 +65 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds dedicated unit tests for core/state/accessors.go. Closes NethermindEth#3688
Give each subtest its own StateDB (and StateReader for history) so they no longer share mutable state or depend on execution order. Rename the contract-writing helper to storeContract to avoid shadowing the package-level writeContract function.
6a35f50 to
b2230fa
Compare
Move accessors_test.go to package state_test so it compiles against the exported accessors only, instead of reaching into unexported internals (stateDB.disk, newTestStateDB). Contract/class tests now write through a plain memory store, and history tests build a StateDB/StateReader via the exported constructors. Drop the "wraps the stored contract on success" case from TestGetStateObject: it only asserted, via unexported fields, that the constructor copied its inputs (a tautological test). The not-found case, which is the actual behavior of GetStateObject, is kept.
b74eedb to
365cedf
Compare
|
@Ehsan-saradar, please fix the lint errors. |
|
Hey @RafaelGranza, fixed the lint error in dca0afe (the long line). Ran make lint locally and it's clean now. Could you re-approve the CI run when you get a chance? Thanks! |
dca0afe to
eb9787a
Compare
RafaelGranza
left a comment
There was a problem hiding this comment.
Please take a look at the copilot comments.
eb9787a to
270cd94
Compare
Description
Adds dedicated unit tests for
core/state/accessors.go, which previously had no direct coverage.Covers:
Has/Get/Write/DeleteContract): absence before write, write→read round-trip, overwrite, and delete.Has/Get/Write/DeleteClass): same lifecycle, round-tripping an encoded Sierra class definition.StateReader— reading at the written block, stepping back to the previous entry, returning zero before the first entry, and honouring block boundaries across two entries.GetStateObject: not-found propagation and successful wrapping of a stored contract.Issue
Closes #3688
Type of change
Testing
go test ./core/state/passes locally.